feat: classify retryable transaction exceptions#10162
feat: classify retryable transaction exceptions#10162memleakd wants to merge 3 commits intocodeigniter4:4.8from
Conversation
Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
|
Thanks for working on this. The goal makes sense. It's useful to flag transaction errors that are worth retrying, like deadlocks and serialization failures. However, I wonder if this should follow the same approach we recently added with Instead of adding a public method like try {
$result = $db->transException(true)->transaction(static function ($db) {
// ...
});
} catch (RetryableTransactionException $e) {
// Retry the whole transaction
}This seems doable because the drivers already inspect native database codes when creating Overall, this looks useful, especially the conservative list of retryable codes. My main suggestion is about the public API shape. I would prefer named exception over |
- Replace the public retryable transaction classifier with a named exception - Classify retryable driver errors through the database exception factory - Update docs and tests for exception-based retry handling Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
|
Thanks a lot for pointing this out. That makes total sense, and I agree it's a better direction. I missed the previous I refactored the PR around that idea. The drivers now classify the known retryable transaction errors and throw One thing I noticed while doing this is that prepared queries still have separate exception paths, similar to the existing unique-constraint behavior. I kept that out of this PR so it stays focused, but I'd be happy to work on prepared-query consistency as a follow-up after this one. |
Also, please update the PR description as well. |
- Route unique constraint and retryable transaction errors through the database exception factory - Move driver-specific unique constraint checks into connection overrides - Add factory coverage for unique constraint exception classification Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
Description
This PR proposes adding
RetryableTransactionExceptionfor driver-specific transaction failures that are reasonable candidates for retrying the whole transaction, such as deadlocks and serialization failures.The goal is to give CodeIgniter a small, conservative, driver-aware classification point without adding another public helper method to database connections. This follows the same direction as
UniqueConstraintViolationException: when the driver recognizes a known database condition, it can throw a more specific exception type instead of a plainDatabaseException.This can already help applications that want to implement their own retry policy:
It is also intended as groundwork for a possible follow-up proposal to add opt-in retry support to the closure-based
transaction()helper. Keeping the exception classification separate makes that later discussion smaller: retryable database errors can be reviewed independently from retry control flow, attempt counts, backoff behavior, and callback side-effect concerns.This PR does not retry anything automatically. It only classifies known retryable transaction failures through a dedicated exception type.
Changes
RetryableTransactionException, extendingDatabaseException.BaseConnection.Driver Policy
The retryable classification is intentionally conservative. It includes transaction-level concurrency failures where retrying the whole transaction is a reasonable default:
121340001,40P0151205,396060,8177It intentionally does not classify lock timeouts, unique constraint violations, resource-busy errors, or SQLite extended busy codes as framework-default retryable transaction failures. Those can be valid retry cases in some applications, but they are more policy-dependent.
References: MySQL, PostgreSQL, SQLite, SQL Server deadlocks, SQL Server 3960, Oracle ORA-00060, Oracle ORA-08177.
Scope Note
This PR focuses on the normal driver query execution paths. Prepared queries still have separate exception paths today, similar to the existing unique-constraint behavior. I kept that out of this PR so it stays focused, but I’m happy to work on prepared-query exception consistency as a follow-up.
Checklist: